Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validate ids prior to using as seed for transmission schedule #14116

Merged
merged 1 commit into from
Aug 20, 2024
Merged

Conversation

ettec
Copy link
Collaborator

@ettec ettec commented Aug 14, 2024

)

// Workflow IDs and Execution IDs are 32-byte hex-encoded strings
func ValidateWorkflowOrExecutionID(id string) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind keeping it in utils.go? Or alternatively moving all the validators to validation.go and their tests to validation_test.go? We are now in a messy state where some methods are here and some there... My preference is to keep stuff in utils.go as it minimizes the total amount of changes needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah crap, yeah I should have at least moved the corresponding tests, my bad. The utils stuff is used outside of the remote package as well now so should sit outside that, I'll tidy it up

Copy link
Collaborator Author

@ettec ettec Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved the validation logic used in remote and other packages in capabilities into validation package along with the corresponding tests. All other logic that is used only in remote package has been kept in the remote/utils.go file.

@ettec ettec force-pushed the ks-404 branch 2 times, most recently from 6afe791 to 8116158 Compare August 19, 2024 15:18
@ettec ettec added this pull request to the merge queue Aug 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 20, 2024
@ettec ettec added this pull request to the merge queue Aug 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 20, 2024
@bolekk bolekk added this pull request to the merge queue Aug 20, 2024
Merged via the queue into develop with commit 7fdc0c8 Aug 20, 2024
127 of 128 checks passed
@bolekk bolekk deleted the ks-404 branch August 20, 2024 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants